Switch to a single routability event #126

Stebalien · 2020-03-04T19:34:59Z

Notes/Questions:

Should we call this "reachability" instead of "routability"?
I've put the constants inside the network package for lack of a better place. I assume we'll want to use them elsewhere. For example, we may eventually want the host to answer the question "do I think this peer can be reached from the public internet?"
Eventually, we'll want something more complex than public/private. We can extend the event when that happens.

This means we can make the event _stateful_ so subscribing always gives us the last state.

willscott · 2020-03-04T19:42:33Z

network/network.go

@@ -53,6 +53,24 @@ const (
 	CannotConnect
 )

+// Routability indicates how reachable a node is.
+type Routability int


worth making this a byte?

@willscott What's the advantage of making this a byte ? Saving space ?

@willscott What's the advantage of making this a byte ? Saving space ?

Right. Could potentially allow for better packing

I'd support it.

No real reason not to.

Hm, well, except for consistency with Connectedness, Direction, and most other enums.

event/routability.go

aarshkshah1992 · 2020-03-04T19:49:25Z

@Stebalien I like "reachability" more than "routability". Makes the concept more readable & understandable.

raulk

+1 to "reachability". It's probably the -ability that I was shooting for, but my mind was fried when this code was written.

event/routability.go

raulk · 2020-03-04T20:16:52Z

network/network.go

@@ -53,6 +53,24 @@ const (
 	CannotConnect
 )

+// Routability indicates how reachable a node is.
+type Routability int


I'd support it.

aschmahmann · 2020-03-05T02:01:16Z

@Stebalien anything preventing this from being merged?

aschmahmann · 2020-03-05T02:02:21Z

network/network.go

@@ -53,6 +53,25 @@ const (
 	CannotConnect
 )

+// Reachability indicates how reachable a node is.
+type Reachability int


Just for clarification is the reason we're put the enum in the network package instead of the event package because we're hoping reachability will be more general than just the EvtLocalReachabilityChanged event?

Yes. Basically, it's not specific to the event itself.

feat: switch to a single routability event

8157087

This means we can make the event _stateful_ so subscribing always gives us the last state.

Stebalien force-pushed the feat/single-routability-event branch from e236267 to 8157087 Compare March 4, 2020 19:35

Stebalien requested a review from raulk March 4, 2020 19:35

Stebalien mentioned this pull request Mar 4, 2020

Stateful eventbus libp2p/go-libp2p-autonat#51

Closed

willscott approved these changes Mar 4, 2020

View reviewed changes

raulk approved these changes Mar 4, 2020

View reviewed changes

rename routability -> reachability

e5f43bb

aschmahmann reviewed Mar 5, 2020

View reviewed changes

Stebalien merged commit d143201 into master Mar 5, 2020

Stebalien deleted the feat/single-routability-event branch March 5, 2020 05:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to a single routability event #126

Switch to a single routability event #126

Stebalien commented Mar 4, 2020 •

edited

Loading

willscott Mar 4, 2020

aarshkshah1992 Mar 4, 2020

willscott Mar 4, 2020

raulk Mar 4, 2020

Stebalien Mar 4, 2020

Stebalien Mar 4, 2020 •

edited

Loading

aarshkshah1992 commented Mar 4, 2020

raulk left a comment •

edited

Loading

raulk Mar 4, 2020

aschmahmann commented Mar 5, 2020

aschmahmann Mar 5, 2020

Stebalien Mar 5, 2020

Switch to a single routability event #126

Switch to a single routability event #126

Conversation

Stebalien commented Mar 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Stebalien Mar 4, 2020 • edited Loading

Choose a reason for hiding this comment

aarshkshah1992 commented Mar 4, 2020

raulk left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aschmahmann commented Mar 5, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Stebalien commented Mar 4, 2020 •

edited

Loading

Stebalien Mar 4, 2020 •

edited

Loading

raulk left a comment •

edited

Loading